Skip to content

ci: smoke-test client CLI path and fix everything-client core drift#346

Open
canardleteer wants to merge 1 commit into
modelcontextprotocol:mainfrom
canardleteer:dev/canardleteer/ci-client-smoke
Open

ci: smoke-test client CLI path and fix everything-client core drift#346
canardleteer wants to merge 1 commit into
modelcontextprotocol:mainfrom
canardleteer:dev/canardleteer/ci-client-smoke

Conversation

@canardleteer

@canardleteer canardleteer commented Jun 17, 2026

Copy link
Copy Markdown

Wire the existing client CLI subcommand into CI (--suite core) and fix two drifts in examples/clients/typescript/everything-client.ts that block core client scenarios (elicitation-sep1034-client-defaults, sse-retry).

Fixes #345.

Motivation and Context

CI runs npm test (vitest) but never exercises the documented client-mode CLI path (node dist/index.js client --command "…"). Regressions in the bundled reference client are only caught when someone runs that path manually against the in-repo fixture (README.md, CONTRIBUTING.md). SDK_INTEGRATION.md documents the same CLI for external SDK conformance clients; it does not validate the in-repo everything-client, so following it would not catch this drift either.

Two concrete examples on current main in --suite core:

  1. elicitation-sep1034-client-defaults — handler registered under the wrong name (elicitation-defaults) and outdated elicitation.applyDefaults capability (should be elicitation.form.applyDefaults).
  2. sse-retry — no handler registered in everything-client.

The scenario implementations and checks appear correct; upstream typescript-sdk's conformance client already registers both handlers with the correct names and capability shapes.

Related (not in scope for #345): #250 proposes eventually removing vendored examples/{clients,servers}/typescript/ and running CI via sdk typescript-sdk@<pinned-sha>. That is not implemented today; this PR is the incremental fix for the current documented workflow.

How Has This Been Tested?

  • npm run build
  • npm test (existing vitest)
  • npm run test:client-smoke (new — core client scenarios via CLI subprocess path, including elicitation and sse-retry)
  • node dist/index.js client --command "npx tsx examples/clients/typescript/everything-client.ts" --scenario elicitation-sep1034-client-defaults — exit 0; all client-elicitation-sep1034-* checks SUCCESS
  • node dist/index.js client --command "npx tsx examples/clients/typescript/everything-client.ts" --scenario sse-retry — exit 0; all client-sse-retry-* checks SUCCESS

Validated on canardleteer/conformance by temporarily enabling the CI workflow on dev branch pushes (not included in this PR — upstream ci.yml only triggers on main or pull_request):

Branch Commit CI run Result
dev/canardleteer/ci-client-smoke (+ temp trigger) c5ec396 + smoke wiring Actions run #27665693080 Passtest:client-smoke 246/246 checks, then vitest
dev/canardleteer/ci-client-smoke-baseline (+ temp trigger, no fixture fix) main + smoke wiring only Actions run #27665749861 Fail on npm run test:client-smoke (exit 1)

Baseline failure (without fixture fixes), from run #27665749861:

Unknown scenario: elicitation-sep1034-client-defaults
Unknown scenario: sse-retry

✗ elicitation-sep1034-client-defaults: 0 passed, 5 failed
✗ sse-retry: 0 passed, 1 failed
Total: 238 passed, 6 failed, 0 warnings

The baseline branch demonstrates that client-smoke CI catches the drift; npm test alone would still pass on main because it never runs the client CLI path.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling (N/A — harness wiring and reference fixture only)
  • I have added or updated documentation as needed

Additional context

  • package.json: add test:client-smoke — runs node dist/index.js client --command "npx tsx examples/clients/typescript/everything-client.ts" --suite core --timeout 60000 (reuses the existing CLI runner per AGENTS.md; no parallel entry point).
  • .github/workflows/ci.yml: run npm run test:client-smoke after npm run build.
  • examples/clients/typescript/everything-client.ts:
    • Register handler as elicitation-sep1034-client-defaults (matches scenario name and CLI MCP_CONFORMANCE_SCENARIO).
    • Advertise elicitation.form.applyDefaults: true (required by @modelcontextprotocol/sdk 1.29+ for default merging).
    • Register sse-retry handler (calls test_reconnection; mirrors upstream typescript-sdk fixture).
  • examples/clients/typescript/elicitation-defaults-test.ts: delete. This was a standalone one-off client for the elicitation-defaults scenario, never referenced by CI, vitest, or the documented client CLI workflow in CONTRIBUTING.md (which runs scenarios via everything-client.ts). That conflicts with repo guidance in AGENTS.md: passing examples belong in everything-client.ts, not separate client files, and unused example clients should be removed. The elicitation coverage now lives in everything-client.ts under the correct scenario key (elicitation-sep1034-client-defaults); keeping the orphan file would leave a second, stale copy still advertising the old elicitation.applyDefaults capability shape.

No scenario or check changes.

Run the documented client subcommand in CI so reference-fixture regressions
are caught before manual CONTRIBUTING runs. Align everything-client with
upstream for elicitation-sep1034-client-defaults and sse-retry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI does not smoke-test client CLI path; everything-client drift on elicitation-sep1034-client-defaults and sse-retry

1 participant